Skip to content

Disable TLS certificate verification in crawler allows MITM/SSRF#2052

Open
omnipotentchaos wants to merge 3 commits into
utksh1:mainfrom
omnipotentchaos:disable-tls-verification
Open

Disable TLS certificate verification in crawler allows MITM/SSRF#2052
omnipotentchaos wants to merge 3 commits into
utksh1:mainfrom
omnipotentchaos:disable-tls-verification

Conversation

@omnipotentchaos

Copy link
Copy Markdown
Contributor

Description

This PR resolves a security vulnerability in crawler.py where httpx.AsyncClient was configured with verify=False and follow_redirects=True, allowing Man-in-the-Middle (MITM) attacks and redirect-based Server-Side Request Forgery (SSRF) to arbitrary internal/external endpoints.

Key changes:

  • Introduced tls_verify: bool = True configuration setting (SECUSCAN_TLS_VERIFY env var) in config.py and documented it in .env.example.
  • Configured httpx.AsyncClient in crawler.py to enforce TLS verification (verify=tls_verify).
  • Replaced automatic redirect following with explicit same-host redirect validation. Redirects pointing to a host different from the original seed target hostname are now blocked with a security warning.
  • Preserved accurate response.history tracking for valid same-host redirect chains.

Related Issues

Fixes #1747

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

How Has This Been Tested?

Ran backend unit tests covering the crawler security policy and helper functions:

pytest testing/backend/unit/test_crawler_security.py testing/backend/unit/test_crawler_helpers.py -v

Copilot AI review requested due to automatic review settings July 21, 2026 07:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Addresses Issue #1747 by hardening the crawler’s HTTP client configuration to prevent MITM risk from disabled TLS verification and to mitigate redirect-based SSRF by disallowing cross-host redirects.

Changes:

  • Adds a new tls_verify setting (env: SECUSCAN_TLS_VERIFY) and documents it in .env.example.
  • Updates crawl_target() to use verify=tls_verify and disables automatic redirects in favor of an explicit same-host redirect policy.
  • Adds new unit tests covering TLS verification usage and redirect allow/block behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
backend/secuscan/crawler.py Enforces TLS verification via config and replaces follow_redirects=True with a same-host redirect loop.
backend/secuscan/config.py Introduces tls_verify setting to control crawler TLS verification behavior.
.env.example Documents SECUSCAN_TLS_VERIFY for configuration discoverability.
testing/backend/unit/test_crawler_security.py Adds unit tests for TLS verify wiring and cross-host redirect blocking policy.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread backend/secuscan/crawler.py
@utksh1 utksh1 added level:advanced 55 pts difficulty label for advanced contributor PRs type:security Security work category bonus label type:bug Bug fix work category bonus label area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests labels Jul 24, 2026

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please narrow and document the redirect policy before merge. The branch combines configurable TLS verification with a new same-host redirect policy; add coverage for relative redirects, redirect-loop/max-hop handling, and the intended behavior when TLS verification is explicitly disabled. Rebase on current main afterward.

@utksh1 utksh1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TLS verification changes are security-sensitive and this branch is behind main. Please rebase and add coverage for the secure default, an explicit opt-out only where documented, and failed verification without changing crawler request semantics beyond TLS handling.

@omnipotentchaos
omnipotentchaos force-pushed the disable-tls-verification branch from b7562c2 to 5fd1196 Compare July 24, 2026 14:39
@omnipotentchaos

Copy link
Copy Markdown
Contributor Author

@utksh1 Please check now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests level:advanced 55 pts difficulty label for advanced contributor PRs type:bug Bug fix work category bonus label type:security Security work category bonus label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[#1] Disable TLS certificate verification in crawler allows MITM/SSRF

3 participants